-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor getReadTheDocsConfig
to use Promise
#286
Conversation
Follow Anthony's suggestion from #64 (comment)
getReadTheDocsConfig
to use `PromisegetReadTheDocsConfig
to use Promise
I found a pattern that I like 👍🏼 . I think it's a clear way to communicate users what's going on, but also leaves the door open to expand checks/validations in the future. I also imagine we could even want to know "What projects are subscribing to the custom event?" in case we need to contact them due to a breaking change or similar. ## Examples #### Calling `event.detail.initialize()` without having defined the `<meta ... />` HTML tag: ![Screenshot_2024-04-09_13-57-20](https://github.com/readthedocs/addons/assets/244656/b1757a08-aa44-4c3b-a5c2-8dbe73a477b4) #### Calling `event.detail.data()` without calling `.initialize()` first: ![Screenshot_2024-04-09_13-56-01](https://github.com/readthedocs/addons/assets/244656/476a934b-7668-4a58-8dbc-8e4254f6049c) Closes #279
OK. I re-structured how we are calling the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, the logic matches what I would expect 👍
@@ -64,73 +67,80 @@ function _getApiUrl(sendUrlParam) { | |||
return url; | |||
} | |||
|
|||
function getReadTheDocsUserConfig(sendUrlParam) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking this out is a good idea 👍 Often, splitting up nested async/promises makes the code easier to follow
Follow Anthony's suggestion from
#64 (comment)